Skip to content

fix(lookup): skip empty cells in approximate MATCH/VLOOKUP/HLOOKUP/XLOOKUP (HF-223)#1697

Merged
sequba merged 14 commits into
developfrom
task/hf-223-match-empty-cells
Jul 16, 2026
Merged

fix(lookup): skip empty cells in approximate MATCH/VLOOKUP/HLOOKUP/XLOOKUP (HF-223)#1697
sequba merged 14 commits into
developfrom
task/hf-223-match-empty-cells

Conversation

@marcin-kordas-hoc

@marcin-kordas-hoc marcin-kordas-hoc commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

What & why

Approximate MATCH, VLOOKUP, HLOOKUP and XLOOKUP returned #N/A (or, in descending mode, the wrong position) when the sorted search range contained genuinely empty cells. Excel 2021 and Google Sheets skip empty cells when computing the lower/upper bound; HyperFormula instead landed on an empty cell during binary search (its EmptyValue Symbol never matched the key). [HF-223]

Root cause

findLastOccurrenceInOrderedRange (src/interpreter/binarySearch.ts): compare() ranks EmptyValue below every value, breaking the sort invariant the binary search relies on, and the typeof foundValue !== typeof searchKey guard then turned a landing-on-empty into #N/A. Shared by approximate MATCH(±1), sorted VLOOKUP/HLOOKUP, and XLOOKUP(searchMode ±2).

Fix

  • The binary search runs directly over the original range while tracking whether the descent ever probes an empty cell. Empty cells are the only source of non-monotonicity in the search predicate, so a descent that never touches one is provably equivalent to a search over the range with empty cells removed and its result is trusted as-is — the common case stays O(log n), including ranges that contain empty cells the descent happens not to touch.
  • Only when the descent probes an empty cell does the search fall back to an O(n) compaction: the non-empty cell indices are collected, the binary search re-runs over the compacted list, and the result maps back to the original index space, so empty cells keep their slots and the matched non-empty cell's original 1-based position is reported unchanged. (In exact-match mode, a hit found by the direct descent is accepted after an equality re-check, skipping the fallback.)
  • The "no match at all" bound cases return the position of the first non-empty cell (never the position of a leading empty cell), and the approximate-bound "next" position steps to the next non-empty index, so skipped empty slots never shift the reported position.
  • AdvancedFind.findNormalizedValue skips EmptyValue on its in-memory ordered path for the same reason, keeping the linear and binary search modes consistent.
  • A matched result cell that is empty is returned as 0 (zeroIfEmptyResult), matching Excel, for VLOOKUP/HLOOKUP/XLOOKUP including multi-cell XLOOKUP return arrays.
  • Empty strings are unaffected (text ranks above numbers, so they still terminate a numeric run).

Performance & edge cases

  • Complexity: O(log n) whenever the binary-search descent probes no empty cell (always the case for gap-free ranges — the typical sorted-lookup workload); O(n) only when an empty cell actually interferes with the descent. A new Sorted lookup benchmark in the performance suite guards this fast path.
  • All-empty range: returns NOT_FOUND directly (the if_not_found result / #N/A), never row 1.
  • Leading empty cells with the key outside the range: the bound modes return the first non-empty position, consistent with the linear search modes on the same data.
  • Duplicates caveat: in exact-match binary modes, when the range contains both duplicates of the key and interspersed empty cells, which duplicate is reported is unspecified (Excel's binary modes likewise leave this unspecified); documented in the function's JSDoc.

Excel / Google Sheets parity

Behaviour verified against the latest Excel and Google Sheets, including exact-vs-blank, empty-string-not-skipped, and the descending early-stop case. For the leading-empty bound cases, HyperFormula's binary modes are consistent with its own linear modes (Excel documents binary search over blank-containing ranges as unreliable).

Tests

Public test suite: handsontable/hyperformula-tests#17 (matching branch task/hf-223-match-empty-cells), including regression locks for the stepping and leading-empty edge cases and a Sorted lookup performance benchmark.

Definition of Done

  • Production code + JSDoc
  • Tests (hyperformula-tests#17, matching branch)
  • Changelog (empty-cell search fix + empty-result → 0 coercion)
  • i18n — N/A (shared-logic fix, no function add/rename)
  • Docs — list-of-differences entry for the remaining Excel divergence

Note

Medium Risk
Changes shared lookup/binary-search logic used by MATCH and all major lookup functions, so incorrect edge-case handling could affect many formulas; scope is limited to lookup parity and is behavior-fix oriented rather than new surface area.

Overview
Fixes HF-223: approximate lookups no longer break when the search range has genuinely empty cells interspersed among sorted values.

Search behavior: findLastOccurrenceInOrderedRange now treats EmptyValue as non-participating in ordering. It keeps an O(log n) binary search when the descent never hits an empty cell; if it does, it compacts non-empty indices and re-searches, with exact-match hits still accepted after an equality re-check. Lower/upper bound paths step to the first/next non-empty index instead of landing on blanks or returning row 1 on all-empty ranges. The in-memory path in AdvancedFind.findNormalizedValue skips empty cells the same way so linear and binary modes stay aligned.

Return values: VLOOKUP, HLOOKUP, and XLOOKUP coerce an empty matched result cell to 0, matching Excel.

Changelog and list-of-differences document the fix and remaining Excel divergence on binary XLOOKUP over blank-heavy ranges.

Reviewed by Cursor Bugbot for commit d3a6f21. Bugbot is set up for automated code reviews on this repo. Configure here.

@netlify

netlify Bot commented Jun 22, 2026

Copy link
Copy Markdown

Deploy Preview for hyperformula-dev-docs ready!

Name Link
🔨 Latest commit d3a6f21
🔍 Latest deploy log https://app.netlify.com/projects/hyperformula-dev-docs/deploys/6a58c6454caa14000834d65e
😎 Deploy Preview https://deploy-preview-1697--hyperformula-dev-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment thread src/interpreter/binarySearch.ts
@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown

Performance comparison of head (d3a6f21) vs base (e0e1261)

                                     testName |   base |    head | change
-------------------------------------------------------------------------
                                      Sheet A | 497.15 |  496.03 | -0.23%
                                      Sheet B | 159.73 |  157.73 | -1.25%
                                      Sheet T |  140.4 |  142.85 | +1.75%
                                Column ranges | 470.87 |  471.42 | +0.12%
                                Sorted lookup |  14348 | 14202.9 | -1.01%
Sheet A:  change value, add/remove row/column |  15.91 |   15.06 | -5.34%
 Sheet B: change value, add/remove row/column | 134.56 |  132.74 | -1.35%
                   Column ranges - add column | 149.04 |  147.35 | -1.13%
                Column ranges - without batch | 458.35 |  447.56 | -2.35%
                        Column ranges - batch | 116.52 |   112.7 | -3.28%

…223)

Excel and Google Sheets ignore genuinely empty cells (but not empty
strings) when computing the lower/upper bound for an approximate match.
HyperFormula instead landed on an empty cell during binary search and
returned #N/A (its EmptyValue Symbol never matched the key), or, in
descending mode, reported the wrong position.

findLastOccurrenceInOrderedRange now runs the ordered search over a
compacted list of non-empty cell indices and maps the result back to the
original index space, so empty cells keep their slots and the matched
non-empty cell's original 1-based position is reported unchanged. When
the range has no non-empty cells the function returns NOT_FOUND, so an
all-empty range yields #N/A for every direction/bound instead of falling
through to the offset-0 branches. The in-memory ordered path in
AdvancedFind.findNormalizedValue skips EmptyValue for the same reason.
Empty strings are unaffected (text is ranked above numbers, so they
still terminate a numeric run). Exact match (matchType 0) is untouched.

Shared by approximate MATCH(+/-1), sorted VLOOKUP/HLOOKUP, and
XLOOKUP(searchMode +/-2). Tests for the public test suite are tracked
separately in handsontable/hyperformula-tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@marcin-kordas-hoc
marcin-kordas-hoc force-pushed the task/hf-223-match-empty-cells branch from c79e10c to ed10ce5 Compare June 22, 2026 23:24
…e JSDoc (HF-223)

Address prep-flip review notes: explain why the empty-cell compaction is
O(n) (correctness requires it; empties make the predicate non-monotonic)
and add a JSDoc block to AdvancedFind.findNormalizedValue for family
consistency with findLastOccurrenceInOrderedRange. Comment/JSDoc only —
no behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@marcin-kordas-hoc
marcin-kordas-hoc requested a review from sequba June 23, 2026 08:07
Comment thread src/interpreter/binarySearch.ts Outdated
Comment thread CHANGELOG.md Outdated
…HF-223)

Per review: the empty-skipping compaction must apply to approximate (bound) lookups
only. Run exact-match mode (ifNoMatch === 'returnNotFound') directly over the original
range so it keeps its O(log n) guarantee and its pre-empty-skip behaviour — an exact
search neither matches a blank nor is redirected past one. CHANGELOG reworded to the
house style (PR link).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@qunabu

qunabu commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Task linked: HF-223 Fix MATCH incompatibility with Excel

@marcin-kordas-hoc
marcin-kordas-hoc requested a review from sequba June 25, 2026 13:44
marcin-kordas-hoc and others added 6 commits July 2, 2026 11:45
…ls (HF-223)

Exact-match binary search (returnNotFound) previously ran directly over the
original range and reported NOT_FOUND when a blank cell sat between the bounds
and the target, hiding a value that is genuinely present. Excel/M365 ground
truth for XLOOKUP(2, {1;2;empty;3}, exact, searchMode 2) is 2, not NotFound.

Add an optimistic fast path: run the direct binary search first (O(log n)); on a
hit return immediately, so the common gap-free case keeps its O(log n) cost. Only
on a miss fall through to the O(n) empty-skipping compaction already used by the
bound modes, which recovers a match an interspersed blank would otherwise hide.

Exact match now reports a hit iff the key is present, independent of ordering
artifacts and gaps. Verified: function-match 82/82, function-xlookup 109/110 (the
one flip is the contested case, now returning the Excel-correct 2), binary-search
20/20.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…LOOKUP (HF-223)

Excel coerces a reference to an empty cell to 0, including a lookup that lands on an empty
result cell. HF returned the raw empty value (null). Mirror Excel on the lookup RETURN value
via a small zeroIfEmptyResult helper at the three return points, matching the existing
zeroIfEmpty coercion already applied to the lookup key.

Verified: XLOOKUP/VLOOKUP/HLOOKUP empty-return-cell -> 0 (Excel-verified 2026-07-02); full
lookup suite still green (vlookup 75, hlookup 41, xlookup 112, match 82, binary-search 20).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mpty (HF-223)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…try (HF-223)

- binarySearch: make explicit the two invariants the empty-skipping fix relies on
  (empties surface as EmptyValue; compare() ranks EmptyValue below all values).
- list-of-differences: note that exact match over a binary-search range is
  gap-independent in HF, whereas Excel's binary modes may return invalid results.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…row 1) (HF-223)

Live-Graph (M365) confirmed a second intentional divergence: on an all-empty
range in a binary search mode, Excel returns the first row's value while
HyperFormula returns the if_not_found result.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@marcin-kordas-hoc

Copy link
Copy Markdown
Collaborator Author

Live-Excel (M365) cross-check. I ran every non-spill HF-223 case through Microsoft Graph against real Excel (one oracle workbook, 30 sheets mirroring the #17 specs, 34 assertions): 32/34 match Excel exactly. The 2 that differ are both intentional "HF is stricter" cases, now in list-of-differences.md:

  • Exact match, binary mode, value after an interspersed empty — HF finds it; Excel returns #N/A/if-not-found.
  • XLOOKUP(…, match_mode 1, search_mode 2) over an all-empty range — HF returns the if_not_found result; Excel returns row 1's value. Our spec asserts "not row 1" on purpose.

One open question for you on that second one: is HF's "return if_not_found, never row 1" the contract we want for an all-empty binary-approx lookup, or should we mirror Excel's row-1 behavior? Happy either way — flagging since it's a deliberate divergence.

(All #N/A cases confirmed too; the M365 account is PL-locale so Excel returns #N/D.)

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 781488f. Configure here.

Comment thread src/interpreter/plugin/LookupPlugin.ts
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
marcin-kordas-hoc added a commit that referenced this pull request Jul 14, 2026
…iew (HF-300)

Review round (sequba + line-by-line + prep-ultra):
- MATCH/VLOOKUP/HLOOKUP: drop the 'HyperFormula skips empty cells when matching
  approximately' claim — that behavior (HF-223 / #1697) is not in this base
  branch, and approximate match returns #N/A on gapped data. Don't overstate.
- INT: shortDescription said 'rounds down' (Excel floor) while the parameter
  description says 'toward zero' — self-contradictory. Reworded to HF's actual
  behavior (integer part, fractional part discarded).
- DATE Year: drop the direct 'Excel' comparison per ADR dec_2 (describe HF
  behavior, not Excel); wording otherwise unchanged.

Source: https://app.clickup.com/t/86caprtgj
ADR: docs/adr/2026-07-13-hf300-function-metadata-enrichment.md
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread CHANGELOG.md Outdated
sequba and others added 3 commits July 15, 2026 16:04
… for out-of-range keys (HF-223)

Address code-review findings on the empty-cell-skipping ordered search:

- Run the binary search directly over the original range while tracking
  whether the descent probes an empty cell. A descent that never touches
  one is provably equivalent to the search over the compacted range, so
  its result is trusted as-is; the O(n) non-empty-index compaction now
  runs only when an empty cell actually interferes. This restores the
  O(log n) cost of sorted lookups over gap-free ranges (and of exact
  binary misses) that the previous revision traded away.
- The no-match bound cases (key below all values in ascending upper-bound
  mode, key above all values in descending lower-bound mode) return the
  position of the first non-empty cell instead of blindly reporting
  offset 0, which pointed at a leading empty cell and disagreed with the
  linear search modes on the same data.
- Document the exact-match duplicate-occurrence caveat and the fast-path
  complexity in the function docs; qualify the Excel-parity claim for
  binary search modes.
- Changelog: note the empty result cell -> 0 coercion in
  VLOOKUP/HLOOKUP/XLOOKUP as its own entry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@marcin-kordas-hoc
marcin-kordas-hoc requested a review from sequba July 16, 2026 11:14
@sequba sequba closed this Jul 16, 2026
@sequba sequba reopened this Jul 16, 2026
@sequba
sequba merged commit fd04f77 into develop Jul 16, 2026
60 checks passed
@sequba
sequba deleted the task/hf-223-match-empty-cells branch July 16, 2026 11:58
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.17%. Comparing base (e0e1261) to head (d3a6f21).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1697   +/-   ##
========================================
  Coverage    97.16%   97.17%           
========================================
  Files          176      176           
  Lines        15450    15483   +33     
  Branches      3417     3429   +12     
========================================
+ Hits         15012    15045   +33     
  Misses         438      438           
Files with missing lines Coverage Δ
src/Lookup/AdvancedFind.ts 98.03% <100.00%> (+0.08%) ⬆️
src/interpreter/binarySearch.ts 99.00% <100.00%> (+0.36%) ⬆️
src/interpreter/plugin/LookupPlugin.ts 97.26% <100.00%> (+0.07%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants